Skip to content

[Win32] Return unrounded value in FontMetrics.getAverageCharacterWidth #2496

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[Win32] Return unrounded value in FontMetrics.getAverageCharacterWidth #2496

wants to merge 1 commit into from

Conversation

wahlbrink
Copy link
Contributor

Fixes: #2461

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 10, 2025 •
edited
Loading

Test Results

  111 files   -  7    111 suites   - 7   11m 48s ⏱️ -5s
4 527 tests  - 56  4 494 ✅  - 72  14 💤  - 3  6 ❌ +6  13 🔥 +13 
  274 runs   - 56    273 ✅  - 53   1 💤  - 3  0 ❌ ±0 

For more details on these failures and errors, see this check.

Results for commit a4c7fee. ± Comparison against base commit 1eabfa3.

This pull request removes 56 tests. 
AllWin32Tests ImageWin32Tests ‑ testDisposeDrawnImageBeforeRequestingTargetForOtherZoom
AllWin32Tests ImageWin32Tests ‑ testDrawImageAtDifferentZooms(boolean)[1] true
AllWin32Tests ImageWin32Tests ‑ testDrawImageAtDifferentZooms(boolean)[2] false
AllWin32Tests ImageWin32Tests ‑ testImageDataForDifferentFractionalZoomsShouldBeDifferent
AllWin32Tests ImageWin32Tests ‑ testImageShouldHaveDimesionAsPerZoomLevel
AllWin32Tests ImageWin32Tests ‑ testRetrieveImageDataAtDifferentZooms(boolean)[1] true
AllWin32Tests ImageWin32Tests ‑ testRetrieveImageDataAtDifferentZooms(boolean)[2] false
AllWin32Tests ImageWin32Tests ‑ test_getImageData_fromCopiedImage
AllWin32Tests ImageWin32Tests ‑ test_getImageData_fromImageForImageDataFromImage
AllWin32Tests TestTreeColumn ‑ test_ColumnOrder
…

♻️ This comment has been updated with latest results.

HeikoKlare
HeikoKlare reviewed Sep 11, 2025
View reviewed changes
Copy link
Contributor

@HeikoKlare HeikoKlare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the proposal! Since FontMetrics is a kind of central class (thus adapting it is critical) some more information about potential impacts (such as on the various consumers in the Eclipse Platform) and how far they were or need to be tested would be good. Also some information on how to test and reproduce changes in behavior would be nice.

@ptziegler fyi, as Draw2d's TextUtilities make use of this method

bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/FontMetrics.java Outdated
*/
public double getAverageCharacterWidth() {
return getAverageCharWidth();
return DPIUtil.pixelToPoint((float)handle.tmAveCharWidth, getZoom());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we add a method converting doubles instead of trimming to float and then converting back?

@wahlbrink
[Win32] Return unrounded value in FontMetrics.getAverageCharacterWidth
a4c7fee 
The value for getAverageCharacterWidth was rounded, even though its
method signature returns a double. The unrounded value provides a better
estimation of the average character width for non-integer zoom factors.

Fixes: eclipse-platform#2461
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HeikoKlare HeikoKlare HeikoKlare left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Win32] FontMetrics.getAverageCharacterWidth should return unrounded value
2 participants
@wahlbrink @HeikoKlare